Implement fallback handler for */resolve requests
#4478
Conversation
fendor
requested review from
joyfulmantis and
michaelpj
and removed request for
wz1000 and
joyfulmantis
| SMethod_InlayHintResolve | ||
| | noResolveData params -> Just params | ||
| SMethod_CompletionItemResolve | ||
| | noResolveData params -> Just params | ||
| SMethod_CodeActionResolve | ||
| | noResolveData params -> Just params | ||
| SMethod_WorkspaceSymbolResolve | ||
| | noResolveData params -> Just params | ||
| SMethod_CodeLensResolve | ||
| | noResolveData params -> Just params | ||
| SMethod_DocumentLinkResolve | ||
| | noResolveData params -> Just params | ||
| _ -> Nothing |
There was a problem hiding this comment.
This pattern matching is bad for long-term maintenance. Anyone an Idea on how to improve this? If we do a string comparison ala isSuffixOf "/resolve" (someMethodToMethodString ...), then I don't see how to actually implement the check, as we need a proof for JL.HasData_ p (Maybe a) where p ~ MessageResult s.
I have an idea for DRY'ing up noResolveData params via Dict, but it introduces a bit of complexity I am not sure is worth it.
There was a problem hiding this comment.
noResolveData params via Dict
Sounds interesting, how do we do it
fendor
force-pushed
the
enhance/plugins/handle-resolves
branch
from
788f389 to
75b1191
Compare
We had multiple reports, where `resolve` requests (such as `completion/resolve` and `codeAction/resolve`) are rejected by HLS since the `_data_` field of the respective LSP feature has not been populated by HLS. This makes sense, as we only support `resolve` for certain kinds of `CodeAction`/`Completions`, when they contain particularly expensive properties, such as documentation or non-local type signatures. So what to do? We can see two options: 1. Be dumb and permissive: if no plugin wants to resolve a request, then just respond positively with the original item! Potentially this masks real issues, but may not be too bad. If a plugin thinks it can handle the request but it then fails to resolve it, we should still return a failure. 2. Try and be smart: we try to figure out requests that we're "supposed" to resolve (e.g. those with a data field), and fail if no plugin wants to handle those. This is possible since we set data. So as long as we maintain the invariant that only things which need resolving get data, then it could be okay. In 'fallbackResolveHandler', we implement the option (2).
fendor
force-pushed
the
enhance/plugins/handle-resolves
branch
from
75b1191 to
86eeb5e
Compare
When resolving CodeActions, CodeLenses or Completions do not have a _data field but a client tries to resolve those items, HLS used to reject this request. To avoid this, we install a fallback handler which returns such items unmodified. We add tests to make sure this works as intended.
|
Added a test for all the resolvable requests that we currently support. Notably, our handlers do not support resolve requests for
How they are handled is similarly hard-coded in https://github.com/haskell/haskell-language-server/blob/master/hls-plugin-api/src/Ide/Types.hs#L996 I don't want to add more features, so I am just adding tests for the supported request types. |
We had multiple reports, where
resolverequests (such ascompletion/resolveandcodeAction/resolve) are rejected by HLS since the_data_field of the respective LSP feature has not been populated by HLS.This makes sense, as we only support
resolvefor certain kinds ofCodeAction/Completions, when they contain particularly expensive properties, such as documentation or non-local type signatures.So what to do? We can see two options:
In 'fallbackResolveHandler', we implement the option (2).
Supersedes #4463 and presumably fixes #4451 and #4467